Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TS] Fix codegen for endpoint returning enums #4427

Merged
merged 8 commits into from
Apr 4, 2024

Conversation

andreaTP
Copy link
Contributor

@andreaTP andreaTP commented Apr 1, 2024

Fix #4426

@andreaTP andreaTP requested a review from a team as a code owner April 1, 2024 12:53
@andreaTP
Copy link
Contributor Author

andreaTP commented Apr 2, 2024

@andreaTP I'm not sure this is the right thing to do here. By the time this function is called, codeclass instances have been converted to interfaces and methods to functions.

@andrueastman I was a little surprised too, but it seems like that this phase of the refiner happens before the transformation you mention.
The it test that I added demonstrates the fix is effective, but I'm not sure it's the most correct approach and/or if I'm missing cases (e.g. should we include method parameters in addition to return types?)

@andrueastman andrueastman requested a review from koros April 3, 2024 07:13
@andrueastman
Copy link
Member

@andreaTP You're right, I've confirmed that the interface generation for the request builders happens after the codeEnumObject creation. So modifying the code class object should be right thing to do here.
I've added @koros as a reviewer as he introduced the codeEnumObject to comment on whether the usings these should be added for method parameters as well.

Otherwise, this looks good from my end.

@andreaTP
Copy link
Contributor Author

andreaTP commented Apr 3, 2024

to comment on whether the usings these should be added for method parameters as well.

🙏 yes please, and for any other case you can think about.

@andrueastman andrueastman merged commit bf2f42c into microsoft:main Apr 4, 2024
207 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[TS] Still open, regression in handling enums
3 participants